Skip to content

feat(rust/cardano-chain-follower): Implement time_to_slot function for Network #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 28, 2024
Merged

feat(rust/cardano-chain-follower): Implement time_to_slot function for Network #91

merged 6 commits into from
Nov 28, 2024

Conversation

@apskhem
Copy link
Collaborator

@apskhem apskhem commented Nov 27, 2024 •
edited
Loading

Description

  • Implemented time_to_slot function and added unit tests for the function.
  • Increased a version from v0.0.4 to v0.0.5.

Related Issue(s)

Closes #89

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@apskhem apskhem added the enhancement New feature or request label Nov 27, 2024
@apskhem apskhem self-assigned this Nov 27, 2024
@apskhem apskhem linked an issue Nov 27, 2024 that may be closed by this pull request
🛠️ [TASK] : Implement the logic for time_to_slot function of the Network type in cardano_chain_follower. #89
Closed
@apskhem apskhem marked this pull request as draft November 27, 2024 08:04
@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

✅ Test Report | ${\color{lightgreen}Pass: 224/224}$ | ${\color{red}Fail: 0/224}$ |

@apskhem apskhem marked this pull request as ready for review November 27, 2024 13:56
stevenj
stevenj requested changes Nov 27, 2024
View reviewed changes
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but we need some tests which use the slot_to_time function to get a time for a slot, and then convert it back to a slot with the new function. They should be equal.

You should also test the other way around, time to slot, and then slot back to time. Again they should be equal.

These tests make sure that we are consistent with the Pallas implementation.

Also, make sure you check a very late time thats in the conway era of the blockchain.

stevenj
stevenj previously approved these changes Nov 27, 2024
View reviewed changes
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@apskhem apskhem requested a review from stevenj November 27, 2024 15:46
@apskhem apskhem added the review me PR is ready for review label Nov 28, 2024
@apskhem apskhem merged commit dbe3d4c into main Nov 28, 2024
25 checks passed
@apskhem apskhem deleted the feat/time-to-slot branch November 28, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkioshn bkioshn bkioshn approved these changes

@stevenj stevenj Awaiting requested review from stevenj

Assignees

@apskhem apskhem

Labels

enhancement New feature or request review me PR is ready for review

Projects

Catalyst
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

🛠️ [TASK] : Implement the logic for time_to_slot function of the Network type in cardano_chain_follower.

4 participants

@apskhem @stevenj @bkioshn